Skip to content

Refresh PR #2216 #3881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 18 commits into from
Closed

Refresh PR #2216 #3881

wants to merge 18 commits into from

Conversation

crazazy
Copy link

@crazazy crazazy commented Nov 12, 2020

Describe your change:

See #2216

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

crazazy and others added 11 commits July 19, 2020 16:37
@crazazy
added a faster solution to project euler problem 14
b893b86
@crazazy
removed blank line
9e62282
@crazazy
switched result type from dict to tuple
7efc169
@crazazy
fixed the naming of python variables
08fe4a9
@crazazy
fixed typos
0cbc3c2 
It's been a while since I worked with typed python, forgive me ok?
@itsvinayak
Update sol3.py
79391d8
@crazazy
un-nested helper functions
e65bb4f
@crazazy
added typehints to all functions
20786f5
@crazazy
added doctests
2118f31 
I am not sure it the calculate_chain doctest works, since they are
testing the side effects of the function, we will see what CI thinks of
it
@crazazy
added the shebang back
12d20ff 
noone gave a good reason on why to remove it, and since this file is
executable anyway (and thus easier to run for me)
@crazazy
removed doctests from calculate_chain
b45a9d0 
doctests reset the environment after each function evaluation, so we
can't use doctests to inspect side effects of void functions
@amaank404
Copy link
Contributor

run the following commands in order:

  • clone your forked repository locally
  • change directory to its root
  • run pip install pre-commit
  • run pre-commit run --all-files twice

commit and push the changes to your fork.

@crazazy
Copy link
Author

crazazy commented Nov 13, 2020

Consider it fixed

@amaank404
Copy link
Contributor

@dhruvmanila, please check this.

@dhruvmanila
Copy link
Member

Thank you for your contribution but currently, we're not accepting solutions to problems that already exist. Try to solve a problem which is not present in the directory and submit a PR for that.

@amaank404
Copy link
Contributor

shall we close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbakdev kbakdev kbakdev approved these changes

@amaank404 amaank404 amaank404 approved these changes

@dhruvmanila dhruvmanila Awaiting requested review from dhruvmanila

@Kush1101 Kush1101 Awaiting requested review from Kush1101

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@crazazy @amaank404 @dhruvmanila @kbakdev @itsvinayak